zetaclient/chains/bitcoin/signer/signer_keysign_test.go (1)

59-63: Strengthen the test by verifying the TSS-signed tx.

After fixing the sighash, validate the witness using the script engine (StandardVerifyFlags) rather than only logging, to assert correctness independent of deterministic k/equality with wallet signature.

zetaclient/chains/ton/observer/outbound.go (1)

55-58: Switch to GetOutboundTrackers: confirm ordering semantics.

The prior call accepted an explicit Ascending order. Please confirm the new API returns a deterministic order (or sort by nonce locally) to avoid reprocessing/skips on restarts.

zetaclient/chains/solana/client.go (1)

12-16: No signature conflicts detected.

• Verified that GetTransaction, GetAccountInfo, GetSlot, and GetSignaturesForAddressWithOpts share identical signatures across observer.SolanaClient, signer.SolanaClient, and repo.SolanaClient.
• Optional: Consolidate the Client interface to embed only repo.SolanaClient (per TODO, see issue #4224) to eliminate redundancy.

zetaclient/chains/bitcoin/client.go (1)

9-13: Unified Bitcoin client interface: LGTM; add doc/mocks to cement the contract

The composition is clean and expresses intent well.

Consider adding a short package doc for the exported type and a mock generator directive to ease testing:

+// Client aggregates Bitcoin capabilities required by common, signer, and observer components.
 type Client interface {
   common.BitcoinClient
   signer.BitcoinClient
   observer.BitcoinClient
 }

zetaclient/chains/sui/observer/inbound.go (1)

126-134: Preserve original RPC error while keeping the sentinel for flow control

Wrap errTxNotFound with contextual details so the underlying RPC error isn't lost.

-    txFresh, err := ob.suiClient.SuiGetTransactionBlock(ctx, txReq)
+    txFresh, err := ob.suiClient.SuiGetTransactionBlock(ctx, txReq)
     if err != nil {
-      return errors.Wrap(errTxNotFound, err.Error())
+      return errors.Wrapf(errTxNotFound, "get tx %s: %v", event.TxHash, err)
     }

zetaclient/orchestrator/preflight_metrics_reporter.go (4)

61-66: Per-chain elapsed time measurement is off; start timer inside the loop

time.Since(start) currently measures from before the loop, not per chain.

-  // perform preflight check
-  start := time.Now()
-  for _, chain := range unsupportedChains {
+  // perform preflight check
+  for _, chain := range unsupportedChains {
+    start := time.Now()
     switch {
       ...
     }
     if err != nil {
       logger.Error().
         Err(err).
         Int64(logs.FieldChain, chain.ChainId).
-        Float64("time_taken", time.Since(start).Seconds()).
+        Float64("time_taken", time.Since(start).Seconds()).
         Msg("unable to report preflight metrics")
     }
   }

Also applies to: 79-85

---

147-150: Inject solana repo/client via dependencies; minor TODO typo

Prefer DI over inline construction to improve testability, and fix the comment typo.

-  // TODO: The Solana repositorty should be injected as a dependency into this function. We
-  // should not have to instantiate the Solana client here.
+  // TODO: The Solana repository should be injected as a dependency into this function.
+  // We should not have to instantiate the Solana client here.

---

136-156: Bound Solana preflight RPC with a per-call timeout

Avoid indefinite waits on flaky endpoints.

 func reportPreflightMetricsSolana(ctx context.Context, app *zctx.AppContext, chain chains.Chain) error {
   ...
-  blockTime, err := solrepo.New(rpcClient).HealthCheck(ctx)
+  localCtx, cancel := context.WithTimeout(ctx, 15*time.Second)
+  defer cancel()
+  blockTime, err := solrepo.New(rpcClient).HealthCheck(localCtx)
   if err != nil {
     return errors.Wrap(err, "unable to get solana last block time")
   }

---

176-191: Likewise, bound Sui preflight RPC with a timeout

Mirror the same defensive timeout for Sui.

 func reportPreflightMetricsSui(ctx context.Context, app *zctx.AppContext, chain chains.Chain) error {
   ...
-  suiClient := suiclient.New(cfg.Endpoint)
+  suiClient := suiclient.New(cfg.Endpoint)
+  localCtx, cancel := context.WithTimeout(ctx, 15*time.Second)
+  defer cancel()
-  blockTime, err := suiClient.HealthCheck(ctx)
+  blockTime, err := suiClient.HealthCheck(localCtx)
   if err != nil {
     return errors.Wrap(err, "unable to get sui last block time")
   }

zetaclient/chains/solana/signer/outbound_tracker_reporter.go (1)

50-60: Honor context cancellation inside the polling loop

Exit promptly when ctx is canceled to avoid orphan goroutines.

   start := time.Now()
   for {
+    select {
+    case <-ctx.Done():
+      logger.Info().Msg("context canceled; stopping tracker reporter")
+      return nil
+    default:
+    }
     // Solana block time is 0.4~0.8 seconds; wait 5 seconds between each check
     time.Sleep(5 * time.Second)

zetaclient/chains/solana/signer/increment_nonce.go (1)

95-123: Instruction construction with sol alias: LGTM; consider returning the interface type

Current return type is *sol.GenericInstruction. If signTx accepts the sol.Instruction interface, prefer returning sol.Instruction to decouple from the concrete type. If signTx requires *sol.GenericInstruction, keep as-is.

Would you like me to draft a follow-up that updates signTx and call sites to accept sol.Instruction if feasible?

zetaclient/chains/solana/signer/signer_test.go (1)

54-88: Explicitly set the chain in every test case

The second, third, and fourth table entries leave chain at its zero value, so base.NewSigner receives a chains.Chain{} with ChainId == 0. That currently works because the constructor only decorates log fields, but it obscures intent and will break if base.NewSigner starts enforcing a non-empty chain (or if metrics rely on the chain metadata). Please initialize the chain field for those cases to match the first entry so the wiring remains explicit and future-proof.

 		{
 			name:         "should create solana signer successfully without relayer key",
+			chain:        chain,
 			chainParams:  *chainParams,
 			solanaClient: nil,
 			tssSigner:    nil,
 			relayerKey:   nil,
 			ts:           nil,
 			logger:       base.DefaultLogger(),
 		},
 		{
 			name: "should fail to create solana signer with invalid gateway address",
+			chain: chain,
 			chainParams: func() observertypes.ChainParams {
 				cp := *chainParams
 				cp.GatewayAddress = "invalid"
 				return cp
 			}(),
@@
 		{
 			name:         "should fail to create solana signer with invalid relayer key",
+			chain:        chain,
 			chainParams:  *chainParams,
 			solanaClient: nil,
 			tssSigner:    nil,
 			relayerKey: &keys.RelayerKey{
 				PrivateKey: "3EMjCcCJg53fMEGVj13", // too short

zetaclient/chains/evm/client/client.go (1)

29-35: Dial options: HTTP client is ignored for ws:// endpoints

rpc.WithHTTPClient applies to http(s) only. If any caller passes ws:// or wss://, dialing may fail or ignore the custom client. Either restrict endpoints to http(s) or branch on scheme.

zetaclient/chains/solana/observer/inbound.go (2)

32-38: Persist initial last scanned signature

You set LastTxScanned in memory only. Consider persisting to DB so a restart doesn’t rescan from genesis for this address.

---

116-117: Update last scanned block even when processing signatures

You only call WithLastBlockScanned when no new signatures are found. Consider updating it after the loop too (when errSlot==nil) to keep metrics fresh.

Additional snippet outside the hunk:

// after the for-loop, before return nil
if errSlot == nil {
	ob.WithLastBlockScanned(lastSlot)
}

zetaclient/chains/solana/signer/withdraw_spl.go (2)

121-124: Reuse recipient ATA from message; avoid recomputation

MsgWithdrawSPL already embeds recipient ATA; recomputing risks divergence and adds an RPC/CPU hit if helpers evolve.

Apply this diff:

-	recipientAta, _, err := sol.FindAssociatedTokenAddress(msg.To(), msg.MintAccount())
-	if err != nil {
-		return nil, errors.Wrapf(err, "cannot find ATA for %s and mint account %s", msg.To(), msg.MintAccount())
-	}
+	recipientAta := msg.RecipientAta()

---

37-51: Fallback path coverage

prepareWithdrawSPLTx uses signMsgWithFallback and createOutboundWithFallback. Consider adding a unit test that exercises the fallback broadcasting path for SPL withdraws. Based on learnings.

zetaclient/orchestrator/bootstrap.go (2)

167-168: Minor: rename alias for clarity.

Consider renaming solbserver to solobserver (or simply solobserver) to avoid confusion at call sites. Behavior is unchanged.

---

161-166: Instrument Solana RPC client with timeouts/metrics via jsonrpc.NewClientWithOpts and rpc.NewWithCustomRPCClient

  httpClient, err := metrics.GetInstrumentedHTTPClient(cfg.Endpoint)
  if err != nil {
    return nil, errors.Wrap(err, "unable to create instrumented HTTP client")
  }
  // create a jsonrpc client with your custom HTTP client
  jsonrpcClient, err := jsonrpc.NewClientWithOpts(
    cfg.Endpoint,
    jsonrpc.RPCClientOpts{HTTPClient: httpClient},
  )
  if err != nil {
    return nil, errors.Wrap(err, "unable to initialize JSON-RPC client")
  }
  // pass the instrumented jsonrpc client into the Solana RPC wrapper
  standardSolanaClient := rpc.NewWithCustomRPCClient(jsonrpcClient)
  if standardSolanaClient == nil {
    return nil, errors.New("unable to create Solana RPC client")
  }

zetaclient/chains/bitcoin/observer/observer_test.go (1)

110-116: Fix typo in test name.

“env var us invalid” → “env var is invalid”.

-      name:           "should fail if env var us invalid",
+      name:           "should fail if env var is invalid",

zetaclient/chains/bitcoin/signer/signer.go (1)

67-71: Lower log level for raw transaction payload.

Hex-encoding entire tx at Info may be noisy; Debug is sufficient while preserving observability.

-  signer.Logger().Std.Info().
+  signer.Logger().Std.Debug().
     Stringer(logs.FieldTx, signedTx.TxHash()).
     Str("signer_tx_payload", hex.EncodeToString(outBuff.Bytes())).
     Msg("broadcasting transaction")

zetaclient/chains/solana/signer/execute_spl.go (1)

111-117: Prefer sol.Meta builder for consistency and clarity when mapping remaining accounts

Constructing *sol.AccountMeta manually is correct but inconsistent with the rest of the file and easy to get flags wrong. Use sol.Meta(pk) and chain .WRITE() when needed.

-remainingAccounts := []*sol.AccountMeta{}
-for _, a := range msg.Accounts {
-	remainingAccounts = append(remainingAccounts, &sol.AccountMeta{
-		PublicKey:  sol.PublicKey(a.PublicKey),
-		IsWritable: a.IsWritable,
-	})
-}
+remainingAccounts := make([]*sol.AccountMeta, 0, len(msg.Accounts))
+for _, a := range msg.Accounts {
+	meta := sol.Meta(sol.PublicKey(a.PublicKey))
+	if a.IsWritable {
+		meta = meta.WRITE()
+	}
+	remainingAccounts = append(remainingAccounts, meta)
+}

zetaclient/chains/solana/observer/observer_test.go (2)

19-27: Add t.Helper() to improve test failure reporting

Mark the factory as a helper to attribute assertion failures to the calling test.

 func MockSolanaObserver(
 	t *testing.T,
 	chain chains.Chain,
 	solanaClient observer.SolanaClient,
 	chainParams observertypes.ChainParams,
 	zetacoreClient interfaces.ZetacoreClient,
 	tssSigner interfaces.TSSSigner,
 ) *observer.Observer {
+	t.Helper()

---

41-51: Resource hygiene: close in-memory DB

Even for in-memory DBs, close handles to avoid descriptor leaks when tests scale.

 	database, err := db.NewFromSqliteInMemory(true)
 	require.NoError(t, err)
+	t.Cleanup(func() { _ = database.Close() })

zetaclient/chains/bitcoin/common/fee.go (2)

60-66: Avoid interface duplication across packages

This BitcoinClient partially overlaps with observer.BitcoinClient (broader surface). Divergent interfaces hinder reuse and mocks.

• Extract a shared zetaclient/chains/bitcoin/client package with cohesive sub-interfaces (Core, Mempool, Blocks, Tx) and consume those here and in observers.
• Alternatively, import the existing broader interface where feasible to avoid a second name for the same concept.

---

264-309: Protect against underflow near chain genesis when looking back blocks

On very short chains (test/dev), blockNumber-i can go negative. Clamp the start height.

-	highestRate := int64(0)
-	for i := int64(0); i < feeRateCountBackBlocks; i++ {
-		// get the block
-		hash, err := bitcoinClient.GetBlockHash(ctx, blockNumber-i)
+	highestRate := int64(0)
+	start := blockNumber - feeRateCountBackBlocks + 1
+	if start < 0 {
+		start = 0
+	}
+	for h := blockNumber; h >= start; h-- {
+		hash, err := bitcoinClient.GetBlockHash(ctx, h)

zetaclient/chains/solana/signer/execute.go (1)

112-119: Avoid MustPublicKeyFromBase58; handle parse errors instead

Using MustPublicKeyFromBase58 can panic on unexpected inputs (e.g., future call sites). You already validate the sender earlier; still, prefer explicit error handling here.

- serializedInst, err := borsh.Serialize(contracts.ExecuteRevertInstructionParams{
-   Discriminator: discriminator,
-   Amount:        msg.Amount(),
-   Sender:        sol.MustPublicKeyFromBase58(msg.Sender()),
+ senderPK, err := sol.PublicKeyFromBase58(msg.Sender())
+ if err != nil {
+   return nil, errors.Wrap(err, "invalid sender pubkey for revert execute")
+ }
+ serializedInst, err := borsh.Serialize(contracts.ExecuteRevertInstructionParams{
+   Discriminator: discriminator,
+   Amount:        msg.Amount(),
+   Sender:        senderPK,

zetaclient/chains/solana/repo/repo_test.go (2)

45-48: Avoid shadowing the imported repo package with a local variable

repo := repo.New(client) shadows the imported package name and hurts readability. Prefer solRepo or r.

- repo := repo.New(client)
+ solRepo := repo.New(client)

Apply similarly in all helper functions.

Also applies to: 65-68, 83-86, 108-111, 126-129

---

76-79: Reduce flakiness from hardcoded first signature

Devnet history can reset; asserting the exact first signature is brittle. Assert non-emptiness or properties instead.

- actualSig := "2tUQtcrXxtNFtV9kZ4kQsmY7snnEoEEArmu9pUptr4UCy8UdbtjPD6UtfEtPJ2qk5CTzZTmLwsbmZdLymcwSUcHu"
- require.Equal(t, actualSig, sig.String())
+ require.NotEmpty(t, sig.String())

zetaclient/chains/solana/repo/repo.go (3)

155-173: Use typed RPCError instead of string matching

Relying on strings.Contains for "-32015" is brittle. Prefer a type assertion to *solrpc.RPCError and check Code.

- switch {
- case err != nil && strings.Contains(err.Error(), errorCodeUnsupportedTransactionVersion):
-   return nil, ErrUnsupportedTxVersion
- case err != nil:
-   return nil, err
- default:
-   return txResult, nil
- }
+ if err != nil {
+   var rpcErr *solrpc.RPCError
+   if errors.As(err, &rpcErr) && rpcErr.Code == -32015 {
+     return nil, ErrUnsupportedTxVersion
+   }
+   return nil, err
+ }
+ return txResult, nil

---

60-101: Default pageLimit when unset and bound it

Guard against callers passing 0/negative pageLimit; also cap to a sane upper bound if desired.

 func (repo SolanaRepo) GetFirstSignatureForAddress(ctx context.Context,
   address sol.PublicKey,
   pageLimit int,
 ) (sol.Signature, error) {
+ if pageLimit <= 0 {
+   pageLimit = DefaultPageLimit
+ }

Apply similar handling in GetSignaturesForAddressUntil.

---

21-26: Unexport DefaultPageLimit or use it internally

The TODO notes this constant need not be exported. Either unexport it (defaultPageLimit) or start using it when pageLimit ≤ 0 as suggested above.

zetaclient/chains/sui/observer/observer.go (1)

34-45: Consider centralizing the Sui client contract

We now have an observer-scoped SuiClient alongside the signer variant, each carrying a slightly different method set. Keeping these interfaces in-package risks drift the next time we add or rename a method. Please consider moving a single shared definition into the chains/sui/client package (or another common location) and importing it here to keep the contract authoritative.

zetaclient/chains/sui/signer/signer.go (1)

32-68: Unify the Sui client interface definition

The signer now owns its own SuiClient interface while the observer keeps a parallel one; both describe the same concrete client but expose different subsets of methods. To avoid divergent contracts and redundant mock maintenance, please consolidate these into a single shared interface (for example in chains/sui/client) and have both signer/observer depend on that canonical type.

zetaclient/chains/evm/signer/signer.go (4)

71-86: Constructor should validate evmClient is non-nil

Prevent nil deref later by validating the dependency at construction time.

 func New(
 	baseSigner *base.Signer,
-	evmClient EVMClient,
+	evmClient EVMClient,
 	zetaConnectorAddress ethcommon.Address,
 	erc20CustodyAddress ethcommon.Address,
 	gatewayAddress ethcommon.Address,
 ) (*Signer, error) {
-	return &Signer{
+	if evmClient == nil {
+		return nil, errors.New("evm client is nil")
+	}
+	return &Signer{
 		Signer:               baseSigner,
 		evmClient:            evmClient,
 		zetaConnectorAddress: zetaConnectorAddress,
 		er20CustodyAddress:   erc20CustodyAddress,
 		gatewayAddress:       gatewayAddress,
 	}, nil
 }

---

355-365: Use SplitN to avoid truncating params containing ':'

Ensures admin command params can include colons.

-		msg := strings.Split(cctx.RelayedMessage, ":")
-		if len(msg) != 2 {
+		msg := strings.SplitN(cctx.RelayedMessage, ":", 2)
+		if len(msg) < 2 {
 			return nil, fmt.Errorf("invalid message %s", msg)
 		}

---

447-454: Add timeout to NonceAt to avoid hanging retries

Wrap the nonce query with the same broadcast timeout to bound each attempt.

-		latestNonce, err := signer.evmClient.NonceAt(ctx, signer.TSS().PubKey().AddressEVM(), nil)
+		ctxNonce, cancelNonce := context.WithTimeout(ctx, broadcastTimeout)
+		defer cancelNonce()
+		latestNonce, err := signer.evmClient.NonceAt(ctxNonce, signer.TSS().PubKey().AddressEVM(), nil)

---

193-201: Parameter ‘chainID’ is unused in newTx

Either use it (re-enable EIP‑1559 path) or drop it to reduce confusion.

Would you like me to provide a minimal diff to remove the param from newTx and its callers, or to restore the dynamic-fee branch behind gas.isLegacy?

zetaclient/chains/ton/observer/observer.go (1)

125-135: Consistent error context in CheckRPCStatus

Message says “TON client health” elsewhere; consider aligning wording for log/search consistency.

-	return errors.Wrap(err, "unable to check TON client health")
+	return errors.Wrap(err, "unable to check TON client health")

Note: If you prefer “RPC health”, update related logs accordingly for consistency across chains.

zetaclient/chains/evm/observer/observer.go (2)

85-102: Constructor should validate evmClient is non-nil

Guard against nil dependencies to avoid runtime panics.

-func New(baseObserver *base.Observer, evmClient EVMClient) (*Observer, error) {
+func New(baseObserver *base.Observer, evmClient EVMClient) (*Observer, error) {
+	if evmClient == nil {
+		return nil, errors.New("evm client is nil")
+	}
 	// create evm observer
 	ob := &Observer{
 		Observer:                      baseObserver,
 		evmClient:                     evmClient,
 		outboundConfirmedReceipts:     make(map[string]*eth.Receipt),
 		outboundConfirmedTransactions: make(map[string]*eth.Transaction),
 		priorityFeeConfig:             priorityFeeConfig{},
 	}

---

141-147: Minor: avoid double OutboundID computation under lock

Compute once to reduce work while holding the mutex.

 func (ob *Observer) setTxNReceipt(nonce uint64, receipt *eth.Receipt, transaction *eth.Transaction) {
 	ob.Mu().Lock()
 	defer ob.Mu().Unlock()
-	ob.outboundConfirmedReceipts[ob.OutboundID(nonce)] = receipt
-	ob.outboundConfirmedTransactions[ob.OutboundID(nonce)] = transaction
+	id := ob.OutboundID(nonce)
+	ob.outboundConfirmedReceipts[id] = receipt
+	ob.outboundConfirmedTransactions[id] = transaction
 }

zetaclient/chains/solana/observer/observer.go (1)

17-67: Unify the SolanaClient interface in a single package

We now have exported SolanaClient interfaces in observer, signer, and repo, each with a slightly different method set. This duplication is brittle—every signature tweak has to be synchronized across packages, and it only takes one missed update for the concrete client to stop compiling or to hide capability mismatches at runtime. Please move the interface to a shared location (for example, a dedicated chains/solana/interfaces package) and have all consumers import that single definition. It keeps the contract authoritative and avoids future drift.

zetaclient/chains/solana/signer/signer.go (1)

51-79: Reuse the SolanaClient interface defined for the observer/repo

The signer declares yet another exported SolanaClient interface, overlapping—but not identical—with the versions in observer and repo. This fragmentation guarantees drift as soon as we add or rename a method. Please factor the interface into a common package (or reuse the one introduced for the observer) so every Solana component depends on the same contract. It simplifies mocking and keeps the API surface coherent.